Skip to content

Prepare for PR to dev/gfdl#6

Open
nikizadehgfdl wants to merge 2047 commits into
nikizadehgfdl:merge-bgc-obc-3_fix_restartfrom
NOAA-GFDL:dev/gfdl
Open

Prepare for PR to dev/gfdl#6
nikizadehgfdl wants to merge 2047 commits into
nikizadehgfdl:merge-bgc-obc-3_fix_restartfrom
NOAA-GFDL:dev/gfdl

Conversation

@nikizadehgfdl
Copy link
Copy Markdown
Owner

No description provided.

nikizadehgfdl pushed a commit that referenced this pull request Aug 12, 2022
Add initialization tests using CS%initialized
@marshallward marshallward force-pushed the dev/gfdl branch 3 times, most recently from 8a66adc to 9de6ce7 Compare September 11, 2023 18:27
Hallberg-NOAA and others added 25 commits July 16, 2025 18:17
  Call MOM_initialize_OBCs() only with the rotated OBC type after the
initialization and potentially rotation of the overall model state in a single
call that is used regardless of whether index rotation is being used, rather
than having two separate calls to MOM_initialize_OBCs() depending on whether
grid rotation is in use.  All answers are bitwise identical.
  Split rotate_OBC_init() into rotate_OBC_segment_fields() and
rotate_OBC_segment_tracer_registry(), and also split out
rotate_OBC_segment_tracer_data() from rotate_OBC_segment_data(), so that the
segment tracer registry can be handled separately and at different points in the
code from the other information on the OBC segment types.  In addition, the
calls to these two new interfaces have been moved much earlier in initialize_MOM
to sit next to other related OBC code.  This is being done in anticipation that
the tracer registries need not be dealt with on the unrotated OBC type when
there is grid rotation and that this will lead to a simplification of the OBC
code and the calls to it.  All answers are bitwise identical, but there are
changes to the public interfaces to the OBC code.
  Eliminate the rotated-index calls to register_temp_salt_segments() and
rotate_OBC_segment_tracer_registry() from iniitalize_MOM() because there is no
longer any use of the segment tracer registries from the unrotated OBC type.
Rotate_OBC_segment_tracer_registry() and rotate_OBC_segment_tracer_data() are no
longer called at all, so they have been eliminated.  This commit also moves
around some OBC-related calls in initialize_MOM to consolidate OBC-related code
blocks, and eliminated the module use statements for some unused routines.  All
answers are bitwise identical, but a publicly visible interface has been
eliminated.
  Added calls to open_boundary_end for the potentially rotated OBC type in
MOM_end() and for the unrotated OBC type after the last time it is used in
initialize_MOM().  The former is standard clean-up at the end of a run, while
the latter will avoid a one-time memory leak when the model is run with
grid-rotation.  All answers are bitwise identical.
  Merged the functionality provided by rotate_OBC_segment_fields() into
rotate_OBC_config() and removed rotate_OBC_segment_fields().  All answers are
bitwise identical but there is one fewer public OBC interface.
  Merged the functionality provided by open_boundary_setup_vert() into
initialize_segment_data() and removed open_boundary_setup_vert().  All answers
are bitwise identical but there is one fewer public OBC interface.
…ded (#936)

* OBC configuration consistency check included

In the subroutine parse_segment_str, a check is now executed to ensure the number of OBC types for each segment is not above 8. The model now stops if more than 8 segments are configured in MOM_input.

* modification of OBC string consistency check

the maximum number of OBC types is now derived from the length of action_str
* Fix issues in MOM_diagnose_KdWork logic

- Fix missing allocation logic for diffusivities needed by idz variables.
- Fix a logic mistake cross-up of ePBL diagnostic id that should have been bkgnd diagnostic id

* Adds code to compute ePBL BBL mstar and bottom boundary layer depth

- Adds output of buoyancy flux from geothermal routine, needed for computing bottom mstar.
- Adds capability into ePBL to use mstar coefficient in BBL, including options to specify the same or different as the surface scheme
- Adds capability to compute bottom mixed layer depth into surface energy based mixed layer depth diagnostic.
- Fixes a discrepency with surface mixed layer depth calculation while preserving old calculation via bug flag.

* Update formatting and rescale ustar_bbl_z_t for non Boussinesq mode in ePBL

* Fix sign error introduced in previous commit

* Wrong ustar BBL was being rescaled in ePBL BBL in prev commit

* ePBL Mstar and bottom MLD commit updates
- ePBL MStar in non Boussinesq scaling updated for converting to ustar in Z_T
- Added more robust PE calculation in MLD calculation with OM4 flag that preserves old answer
- Referenced bottom MLD claculation potential density to bottom pressure instead of surface pressure.

* Fixed sign error in PE anomaly bottom MLD calculation

---------

Co-authored-by: brandon.reichl <brandon.reichl@noaa.gov>
  Add the new runtime option MASK_COASTAL_PRESSURE_FORCE to use land masks to
zero out the diagnosed barotropic pressure gradient accelerations at coastal or
land points.  If it is enabled, this changes diagnostics and it improves the
reproducibility of certain debugging checksums, but it does not alter the
solutions themselves.  By default, all answers and diagnostics are bitwise
identical, but there is a new runtime parameter in some MOM_parameter_doc files.
* Update to allow reproducibility for NMME initialization runs.

Uses DEFAULT_ANSWER_DATE <20190101 to reproduce forecast results from older executable.

* Correcting trailing spaces.

* (*) Update to allow for reproducibility of NMME results.

Will change logfiles due to new parameter (REPRODUCE_2018_NMME_ANSWERS) being added.

* Broke two lines into continuation lines due to length exceeding 132 characters previously.

* Fixed trailing spaces
Flags `do_advection` and `do_diabatic` in subroutine step_MOM are
simplified for slightly better readability and efficiency.
If FRAZIL=False tv%frazil is not allocated but its halo might still
be updated in post_diabatic_halo_updates, which will fail.

If FRAZIL=True, no answers are changed.

If FRAZIL=False, no answers from before this bug was introduced are changed.
update MOM6 to its main repo. 20250801 commit
  Updated the default values of 15 runtime parameters, as agreed upon in a MOM6
consortium wide conversations on July 29, 2024 and December 16, 2024.  The most
prominent of these is that the default equation of state is now EQN_OF_STATE =
"WRIGHT_FULL", in place of the buggy previous default of "WRIGHT".

  The 8 answer date parameters REGRIDDING_ANSWER_DATE, TIDES_ANSWER_DATE,
WAVE_INTERFACE_ANSWER_DATE, MEKE_GM_SRC_ANSWER_DATE, NDIFF_ANSWER_DATE,
KPP%ANSWER_DATE, HOR_DIFF_ANSWER_DATE and LOTW_BBL_ANSWER_DATE all now take
their default values from DEFAULT_ANSWER_DATE.

  The bug-retention parameters MEKE_GM_SRC_ALT_SLOPE_BUG, HOR_DIFF_LIMIT_BUG,
BACKSCATTER_UNDERBOUND, DETERMINE_TEMP_CONVERGENCE_BUG, LA_MISALIGNMENT_BUG and
IDL_HURR_SCM_EDGE_TAPER_BUG are now false by default.

  The MOM_input files for the test cases in the `.testing/tc[01234]` directories
were updated to explicitly set all of these parameters to their previous default
values if they are used in the relevant cases, as well as parameters that will
be change following discussions from June 2, 2205.

  Bitwise identical answers are recovered if all 15 of these parameters are set
explicitly, but answers can change if any of these parameters take default
values.  All of these default changes are the consensus decision of
consortium-wise MOM6 dev calls.
  Updated the default values of 6 runtime parameters, as agreed upon in a MOM6
consortium wide conversations on June 2, 2025. VISC_REM_BUG and FRICTWORK_BUG
are now false by default. MASS_WEIGHT_IN_PRESSURE_GRADIENT_TOP now takes its
default value from the setting for MASS_WEIGHT_IN_PRESSURE_GRADIENT.  Similarly,
the default for DRAG_DIFFUSIVITY_ANSWER_DATE is now set to follow
SET_DIFF_ANSWER_DATE. KELVIN_WAVE_VEL_NUDGING_TIMESCALE is now set to a negative
value by default, forcing the user to provide a valid value at runtime.
SHELFWAVE_CORRECT_AMPLITUDE now defaults to True.
  Obsoleted 7 runtime parameters that always take the same value, as agreed upon
in a MOM6 consortium wide conversations on June 2, 2025, and added tests to
catch these parameters in MOM_obsolete_params.  The parameters that were
obsoleted include BETTER_BOUND_KH, BETTER_BOUND_AH, USE_DIABATIC_TIME_BUG,
FIX_UNSPLIT_DT_VISC_BUG, CFL_BASED_TRUNCATIONS and KD_BACKGROUND_VIA_KDML_BUG.
The runtime parameter MAXVEL still exists, but it is logged in a different order
than before, changing the contents of the MOM_parameter_doc files. All answers
are bitwise identical in cases that run, but cases that use these parameters
will experience a fatal error.
* (*) Add option for minmod limiter for RayTracing

This PR adds a minmod limiter option for the advection
of the internal tides energy, which becomes the new default.
The current positive definite scheme is kept as an option
but has shown to create ripples at the grid scale.

* change adv_limiter options from string to integer

---------

Co-authored-by: Raphael Dussin <Raphael.Dussin@noaa.gov>
* (*) Multiple fixes for the ray tracing

- Solve the issue of rays not propagating through the northfold: the
use of pass_vector for speed_[x/y] is not appropriate since these arrays
are meant to be scalar and the direction is contained in the angle dimension
of energy. changing to regular pass_var at the appropriate cell location fixes it

- The energy gets trapped at critical latitude: this PR introduces 2 options for
energy propagation, either propagate along the critical latitude or reflect on it.
These are controlled by TURN_CRITICAL_LAT (False: get trapped, True: do something)
and REFLECT_CRITICAL_LAT (True: reflect like a solid wall, False: propagate along)

- Several divisions by constant number were eliminated

* (*) Multiple fixes for the ray tracing

- Solve the issue of rays not propagating through the northfold: the
use of pass_vector for speed_[x/y] is not appropriate since these arrays
are meant to be scalar and the direction is contained in the angle dimension
of energy. changing to regular pass_var at the appropriate cell location fixes it

- The energy gets trapped at critical latitude: this PR introduces 2 options for
energy propagation, either propagate along the critical latitude or reflect on it.
These are controlled by TURN_CRITICAL_LAT (False: get trapped, True: do something)
and REFLECT_CRITICAL_LAT (True: reflect like a solid wall, False: propagate along)

- Several divisions by constant number were eliminated

* add call to turning latitude in propagate_x

This should satisfy the rotational symmetry.

As expected, this has no impact on global case
since reflected energy from propagate_x then
does not need to be reflected in propagate_y.

* update units order

---------

Co-authored-by: Raphael Dussin <Raphael.Dussin@noaa.gov>
When debugging layout non-reproducible problems, this hchksum showed
up as having changes in the halo BUT it is immediately followed by
a pass_var(). Either we need to only check the C-domain or do hchksum
with halos after the pass_var().
The rate at which eta_src was added to eta within the barotropic
solver allowed the instantaneous eta to ground-out and drop below
the bottom during the extend steps over which the filtered state
is calculated. eta_src was previously adjusted so that it could
at worst ground out over the course of the baroclinic step, but
did not allow for the extra time needed when filtering.

This change was found to be helpful when debugging problems in
vanished layers under a grounded thermodynamic ice shelf.
An adjustment to the barotropic mass source used in the correction step
of RK2 was making matters worse for layers under a grounded ice shelf.
This adjustment essentially does not do anything for the deep ocean
(i.e. was tiny tiny) so we suspect this was a hold over from the earlier
BT algorithms.
The BT solver regularly spits out warnings about the SSH dropping below
the ocean bottom, particularly frequently for extreme scenarios such as
the vicinity of the ground line in an ice shelf cavity. These changes
apply a limiter on the time-integrated transport within the BT solver
that based on the initial volume adjusted for any sinks.

- Added run-time flag BT_LIMIT_INTEGRAL_TRANSPORT that turns on
  calculation of the initial and projected available volume.
  By default, this is off and the default available volume is huge.
- Implemented a limiter without `if`s for efficiency.
- Added a FATAL test to check that the initial conditions do not already
  have negative total thickness.
- Flipped the WARNING about negative total thickness to be FATAL when
  using the limiter, since if the limiter is not robust then we've done
  something wrong again.
- Extended check on number of steps for BT solver, wrt filtering steps
  - Changed an ==0 to <=0, since we encountered a weird situation where
    it happened (unknown cause)
Adds analogous code to MOM_dynamics_split_RK2b.F90 that is identical to the
code that was added to MOM_dynamics_split_RK2.F90 to avoid readjusting the
barotropic mass source to try to reconcile the baroclinic and barotropic sea
surface heights.  By default all answers are bitwise identical, but the runtime
parameter BT_ADJ_CORR_MASS_SRC will not appear in the MOM_parameter_doc files
for cases with SPLIT_RK2B set to True.
adcroft and others added 30 commits April 22, 2026 13:05
Under optimization -O2 with ifort we had some floating-point divergences
in the new Recon1d schemes. Two schemes diverged from their counterparts
that use the old "select" pathways. One scheme failed self-consistency
tests due to inlining allowing some aggressive optimization to occur.

Recon1d_PPM_H4_2019:
- Add explicit parentheses in end_value_h4() to enforce evaluation order
  and ensure bit-reproducibility across compilers.

Recon1d_PPM_hybgen:
- Refactor reconstruct() into a cleaner two-pass approach using new
  private helpers (hybgen_ppm_coefs, bound_edge_values,
  check_discontinuous_edge_values) copied from phased-out modules to
  avoid external dependencies. Bit-for-bit with the old PPM_HYBGEN scheme.
- Add average() override that clamps sub-cell averages to
  [min(ul,ur), max(ul,ur)], reproducing force_bounds_in_subcell behaviour.

Recon1d_PLM_WLS:
- Fix LS_error(): remove 0.5/0.25 factors and reformulate as squared
  deviation from the optimal slope.
- Also updated Doxygen math which had some errors.
- In check_reconstruction(), copy state via assignment (perturbed = this)
  since an inline call to %reconstruct() on same data was yielding LSB
  differences.
* More fixes to achieve consistent rotation with FMAs

* New Krylov methods for ice-shelf velocity

Added MINRES and CR methods to solve the shallow shelf approximation.
Further optimized CG. The new CG is answer changing, but only because
IDIAGU/IDIAGv reciprocals are now procomputed to replace per-iteration
divisions with multiplications. MINRES and CR can be faster than CG.
Also added CG_HALO_SHRINK parameter to disable the halo-shrinking
scheme for CG; combined with the other changes, this reduces the
number of pass_vector calls to once per iteration, rather than 4
pass_vector calls every nihalo iterations. Disabling the
halo-shrinking can be faster for certain combinations of grid
properties (halo size, symmetric/nonsymmetric, grid size).

* Remove unneeded line

Removed a line that caused an error when compiling in debug mode,
where a variable was being set to the value of another variable that
was not set yet. This line was not needed.

* Add wrapper for ice-shelf Krylov methods

Added a wrapper subroutine that performs the shared setup for each of
the Krylov methods, calls the selected Krylov method, and assigns
BCs. Also: made sure the number of iters is returned from each method
in the unlikely case of an early exit; slight change to where CG
convergence is tested to avoid evaluating extra unneeded code.

* Fix ice-shelf inner solver units. Change exit point for MINRES for efficiency.

* Fix units for residual scaling within inner solvers

* Fix formatting
* Add register restart for taux_bot and tauy_bot with SPLIT_BOTTOM_STRESS
- taux_bot and tauy_bot need to be in the restart file when using SPLIT_BOTTOM_STRESS, but they presently are not.
- This commit adds them to the restart files in MOM_dynamics_split_RK2.

* Adds restart taux(y)_bot capability from RK2 to RK2b

* Fix formatting issues in MOM_dynamics_split_RK2b.F90

* Fixes for allocating bottom stress in static memory
- As suggest by R. Hallberg and copilot, the ALLOC_ macros should not have been used on taux/y_bot, so it has been replaced with explicit allocate statements.
- Fixed some minor formatting issues caught by copilot.

* Fix issue with deallocating taux/y_bot

---------

Co-authored-by: brandon.reichl <brandon.reichl@noaa.gov>
Co-authored-by: Alistair Adcroft <adcroft@users.noreply.github.com>
  Refactored downsample_field_3d() and downsample_field_2d() and added the
option to use a rotationally symmetric sum when downsampling diagnostic output.
The new runtime parameter SYMMETRIC_DOWNSAMPLE_SUMS, triggers the use of a
symmetric order of the sums, either directly or for downsampling by a factor of
4 or more via a call to symmetric_sum().  Internally the refactoring stores the
values that are to be summed in a 2-d or 1-d array before calling the new
internal routine square_sum() or sum_1d().  There are also two comments added
noting where thicknesses are used at velocity points without averaging to
correct for the difference in their staggered locations.  By default, all
solutions and diagnostics are identical, although there is annoying change in
the locations of negative zeros for some diagnostics.  However, when
SYMMETRIC_DOWNSAMPLE_SUMS is set to true, answers are mathematically equivalent
but there are changes at roundoff in diagnostics that are written at 64-bit
precision.
Add AI-assistance policy, agent instructions, and code style docs
This commit addresses an unintentional answer change introduced in
commit 1e9ac39. Subroutine initialize_OBC_segment_reservoirs misses the
flag OBC%update_OBC_seg_data, which would be false during
initialization (maybe a bug itself). This flag defers the BGC tracer
initialization to subroutine fill_obgc_segments, which is called later.
The get_param call for DT_OBC_SEG_UPDATE_OBGC was placed before
MOM_initialize_fixed(), so OBC_in was always NULL() at that point. The
guard do_not_log=.not.associated(OBC_in) therefore suppresses the
parameter from MOM_parameter_doc even in runs with active open boundary
conditions.

Fix by moving the call to after OBC_in is set, right after
MOM_initialize_fixed.
Reworked the SSA FEM shape functions to ensure bitwise identical answers under rotation
  The tracer advection diagnostics had been accumulated using values of the do_i
masks that were not initialized, but this had no logical impact because the
tracer fluxes themselves were zero at any points where the value of the masks
might matter.  To address this, the unnecessary masks were eliminated, instead
relying on the fact that the advective tracer fluxes are already 0 at any points
that would ever be masked out.  Once merged in, this commit will have addressed
the issue noted at github.com//issues/952, which can then be
closed.  All solutions and diagnostics should be bitwise identical (apart from
the annoying possibility of changes in negative zeros).
  Added the optional runtime parameter may_reset to MOM_set_verbosity() to help
regulate when the verbosity can be changed in repeated calls on the same PEs.
Also added a new module variable, verbosity_set, to the MOM_error_handler module
to record whether the verbosity has already been set by a call to
MOM_set_verbosity().  The valid range for verbosity in MOM_set_verbosity was
increase from [1 to 9] up to [0 to 9] to match the description of verbosity in
various comments. In addition, the primary call MOM_set_verbosity() in
initialize_MOM() now includes the new argument with the value that will cause
the MOM6 setting for VERBOSITY to take precedence over any values coming from
other components (like SIS2 in some cases) that also use MOM_error_handler code
and run on the same PEs.  Also extended the description of the VERBOSITY
parameter to note the setting that causes the call tree messages to be printed.
All answers are bitwise identical, but there is a new optional argument to a
publicly visible routine.
* Diag buffer: Set fill_value_3d ks/ke size

The `fill_value_3d` unit test of `diag_buffer_unit_tests_3d` was not
setting its vertical extent, causing inaccurate allocation sizes and
potential segmentation faults.  (Or, in my case, complete memory
consumption and OS meltdowns.)

This patch adds `set_vertical_extent` and uses the same 10-element
width.  This appears to fix the issue for me.

* Diag buffers: Streamline testing

Some unused variables and incorrect comments have been removed from the
unit testing.

Assisted-by: copilot
`cuberoot()` uses the `modulo()` intrinsic to compute the exponent of
the rescaled value of its input `a` in the range [0.125,1).  However,
there are platforms (e.g. NVIDIA GPUs) which do not implement
`modulo()`.

Review of this function also revealed some unnecessary complexity which
was simplified by using the `sign()` intrinsic.

This patch updates the `rescale_cbrt()` implementation, and also
documents the underlying mathematics more carefully.
  Converted the internal subroutine add_xyz_method() in MOM_diag_mediator into
the function xyz_method() for greater generality and code-reuse.  It no longer
takes a diag_type argument, instead working directly with the contents of its
axes_grp type argument.  All answers are bitwise identical, and all publicly
visible interfaces remain the same.
  Set downsampled masks using the same stencils as is used for the masked
downsampling of the underlying data.  This required extensive revisions to the
downsample_mask_2d() and downsample_mask_3d() routines, including a new method
argument to indicate how the data is to be downsampled analogous to the argument
that was already being provided to the downsample_field routines.  All solutions
and primary diagnostics are bitwise identical, but this commit corrects a
glaring inconsistency in the masking of downsampled diagnostics.
  Eliminated the MSK downsampling method and the MSK option code in the
downsample_field() routines because this option did not take in to account the
points that are actually used in downsampling fields that are not at tracer
points.  Instead the downsampled mask is determined by calling
downsample_field() on an array of ones to determine where there are unmasked
downsampled points.  This change only applies in cases where an optional mask
argument is being supplied to a post_data() call (as is the case for some static
fields).  As post_data() is currently used in MOM6, a mask argument only is
passed in for diagnostics at tracer points so it is possible that there are not
actually any instances where this will change any answers.
  At one point, fill_miss_2d had an optional `prev` array that could be used to
fill in values that were not horizontally connected to other points at the same
level, and it issued a warning when that array was not provided.  However,
`prev` was always being provided, so in 2022 as a part of some more extensive
refactoring and clean-up of MOM_horizontal_regridding.F90 this optional argument
was made mandatory, and the extra logical test around the warning block became
identical to the first.  As noted in github.com//issues/929, this
lead to a block of code that would clearly never be reached.  This extra block
of code has now been removed.  All answers are bitwise identical.
  Added an else branch in thickness_diffuse_init to set CS%Use_KH_in_MEKE to
false when USE_MEKE is false, thereby avoiding the possible problems with
segmentation faults due to an uninitialized variable, as documented in
github.com/mom-ocean/issues/1696. It will remain a fatal error if
`USE_KE_IN_MEKE` is in an input file but `USE_MEKE = False` and
`FATAL_UNUSED_PARAMS = True`.  All answers that worked previously will be
bitwise identical, but this could fix problems with a segmentation fault arising
from an uninitialized logical variable.
* Change loop order from m-[ij]-k to m-k-[ij].
* Rename u_L/v_L to L_in/L_out for clarity.
* Add flux_to_res and face_area temporary variables for efficiency
* Hoist dir and OBC%tres_[xy] writes outside the tracer loop.
* Remove unused local variable ntr.
Copy resrv_lfac_in/out from segment%field (looked up by fd_id at
runtime in update_segment_tracer_reservoirs) into the segment tracer
registry. The copy can be populated via new optional arguments added to
register_segment_tracer, which is only used by BGC tracers.

resrv_lfac_in/out is used in subroutine register_segment_tracer, which
updates tracer reservoir. Logically, segment%field is the "raw" data,
internal to OBC module, while segment%tr_Reg is what is seen by the rest
of the model. Storing resrv_lfac_in/out in the registry frees the need
of segment%field data in update_segment_tracer_reservoirs.

The change eliminates the need to use fd_id to identify whether
resrv_lfac_in/out should be the default (1.0). Correspondingly,
fd_index optional arguments is removed from register_segment_tracer,
as its no longer needed.

The change also paves the way to use resrv_lfac_in/out to fold the
special uniform concentration case in the upcoming commit.
  Modified the offline tracer code to always allocate the offline mixed layer
depth when offline tracers are being used, and set it to HMIX_FIXED when
READ_MLD is false.  In addition, MLD_VAR_NAME is now passed to
update_offline_from_files(), whereas it was previously just being set and
ignored.  As a follow up, it might be useful to add the option of setting the
mixed layer in offline tracer mode dynamically using diagnoseMLDbyEnergy() or
diagnoseMLDbyDensityDifference(), but that is beyond the scope of this commit.
This commit addresses the bugs identified at
github.com//issues/1100.  All answers are bitwise identical in all
cases that worked previously, but some offline tracer cases that had been
failing previously with a segmentation fault should work properly now.
…nservation, and adding time integration (#1088)

* Add ability to use different ML depths with brine plume

Codes in 3 options, MLD_003, MLD_EN1, and H_EPBL, all of which can be passed to brine plume param to set the plume depth

* Update brine plume scheme to conserve salt and time integrate

- Brine plume scheme failed dimensional consistency because it was not multiplied by the time step.  The units of plume_flux in the description are updated, which exposes why it failed the time dimension consistency test.
- The brine plume scheme was moved after the application of other fluxes that can add salt/heat/mass to the ocean in applyboundaryfluxesinout.  This change made it easier to track that it only moves salt in the vertical and doesn't create/destroy salt.
- The brine plume salt flux is now multipled by dt to convert from a rate to an amount.  This has a large impact on the behavior, but is approximately compensated by an unrealated bug in SIS2.
- The brine plume scheme did not conserve salt because it could neglect salt if the mixed layer depth wasn't deeper than the center of the bottom grid cell, possibly due to round off.  A new condition is added to insert salt into the bottom layer if it is thick enough to accept salt.  This significantly improves conservation, though round-off level errors are still possible.
- Complete a stub for a brine plume diagnostic, which can be used to verify how the brine plume scheme moves salt in the vertical.
- Adds an option to increase the vertical scale for brine plumes proportionally to the chosen mixed layer depth.
- Updated some parameter names and formatting related to the MLD used in the brine scheme.

* Fix OMP directives for brine plume helpers in MOM_diabatic_aux

* Updates for conservation in brine plume parameterization

- The brine plume salt flux contribution is no longer added to the top level and subtracted later in the brine plume part. Rather, the salt that is added in applyboundaryfluxes now has the brine plume salt flux removed.  The salt flux is then simply applied by adding it back in during the brine plume part.
- The code to handle flux deposition within the mixed layer was rewritten to ensure the salt flux is entirely used by the bottom-most finite thickness layer.
- The flux is now prescribed by analytical integral over the layer instead of midpoint quadrature.
- Debugging options are enhanced to better track and report on salt conservation.

* Make sure salt added/removed in brine plume is always computed

* Fix formatting and OMP directive

* Clean up brine plume commit, standard out, and debug options

* Fix OMP associated with brine plume

* Fix OMP associated with brine plume (again)

---------

Authored-by: brandon.reichl <brandon.reichl@noaa.gov>
  This commit provides the MOM6 diag_mediator with information about the
locations and orientations of any open boundary condition segments at velocity
points, and then uses this to select the correct thicknesses to use for vertical
remapping of diagnostics onto alternative diagnostic grids (e.g., z or rho2).
It also makes a change to properly interpolate the thicknesses that are used as
weights for downsampling velocities.  Previously a one-sided estimate of the
thicknesses was being used, which did not satisfy rotational consistency.

  The specific changes here include:

 . The addition of the new publicly visible routine diag_mediator_set_OBC_info()
   and a call to this routine from initialize_MOM() when OBCs are being used.
   This routine stores information about the location and orientation of OBC
   segments.  It does not need to be called when OBCs are not being used.

 . The addition of two new integer arrays to the diag_ctrl type with values of
   0, -1 or 1, depending on the presence and orientation of OBCs at velocity
   faces.

 . The addition of OBC_u and OBC_v arguments to 3 publicly visible routines in
   MOM_diag_remap (diag_remap_do_remap(), vertically_reintegrate_diag_field()
   and vertically_interpolate_diag_field()) and to their private counterparts
   (do_remap vertically_reintegrate_field() and vertically_interpolate_field()).
   These arrays are used to select the appropriately interpolated thicknesses
   at velocity points to use as weights for remapping.

  In tests of these changes in the loop current test case, the changes to the
remapped velocities and transports at OBC points are subtle but real, while in
the interior (away from OBCs) the diagnostics are unaltered.

  All solutions are bitwise identical and no primary diagnostics are changed,
but there are changes to remapped diagnostics in cases with open boundary
conditions.  Within this commit this a new publicly visible subroutine, several
changes to the interfaces of 3 publicly visible routines and the addition of two
new 2-d arrays to the diagnostics control structure.
* *Add framework support for ANN mesoscale streamfunction

Extend MOM_isopycnal_slopes to output density gradients (drdx_u,
drdy_v, drdz_u, drdz_v) as optional arguments from
calc_isoneutral_slopes. Extend MOM_thickness_diffuse to accept
externally computed streamfunctions (Sfn_unlim_u_3D, Sfn_unlim_v_3D)
in thickness_diffuse_full. Wire up the new ANN streamfunction module
in MOM.F90 with initialization, computation, and restart calls.

* *Add ANN-based mesoscale streamfunction module

Introduces MOM_meso_sfn_ANN, which predicts density-flux-based
mesoscale streamfunctions using a neural network. Features include
configurable stencil windows for local density and velocity gradient
inputs, runtime-tunable clamping thresholds, boundary proximity
masking, and a fallback GM_simple mode for testing. The ANN outputs
buoyancy fluxes that are converted to streamfunctions via the
local density gradient magnitude.

This is answer-changing only when USE_THICKNESS_DIFFUSE_ANN=True.
The parameter defaults to False, so no existing experiments are
affected.

* *Clean up ANN mesoscale streamfunction review feedback

Address Pavel Perezhogin's review and failing CI checks.

Review:
- Swap ANN_apply_vector_orig -> ANN_apply_vector_oi (faster path)
- Gate MOM_thickness_diffuse limiters on use_meso_sfn_ANN so the
  dev/gfdl baseline is unchanged when the ANN is off
- Refactor the streamfunction clamp to act on the velocity-scale
  Upsilon (Ferrari et al. 2010) before multiplication by dy_Cu/dx_Cv,
  giving a grid-independent cap. Rename the runtime parameter
  MESO_SFN_SFN_CLAMP to MESO_UPSILON_CLAMP and shift the default
  from 1.0e6 volume-transport units to 1.0e4 m2 s-1 velocity-scale.

Unit cleanup:
- Density flux variables (Fx_u, Fy_v, Fx_c, Fy_c) relabeled from
  [R L-1 ~> kg m-4] to [R L T-1 ~> kg m-2 s-1]
- Volume streamfunction dummy args sfn_u/sfn_v relabeled
  [L2 T-1] -> [Z L2 T-1 ~> m3 s-1] to match Sfn_unlim_u_3D
- mag_grad / mag_grad_floor switched from [R L-1] to [R Z-1]
  (the computation already produces [R Z-1] via the Z_to_L rescale
  of drdx_u; the floor comparison only works correctly under
  internal rescaling when both sides share the same internal unit)
- Fix conversion= factors on all 11 meso_sfn_* diagnostic fields;
  previous values were copy-pasted from sfn_unlim_x and did not
  match the registered quantities
- Add scale= argument to mag_grad_floor and MESO_UPSILON_CLAMP
  get_param calls for rescale-safe behavior
- Replace MESO_SFN_GRAD_MAG_EPSILON parameter with derived
  h_neglect-style locals rho_grad_neglect [R L-1] and
  vel_grad_neglect [T-1] (1e-30 scaled per dimension)

CI fixes:
- build-openmp: add present_drdx_u/drdx_u/drdz_u and
  present_drdy_v/drdy_v/drdz_v to the two !$OMP parallel do
  shared(...) clauses in calc_isoneutral_slopes
- check-style-and-docstrings: split multi-variable id_* declarations
  in MESO_SFN_ANN_CS so each has its own doxygen !< comment

Answer-changing only when USE_THICKNESS_DIFFUSE_ANN=True.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* *Audit cleanup and CI fixes on meso_sfn_ANN

Act on the broader audit of MOM_meso_sfn_ANN and fix the two
remaining CI failures.

Audit-driven changes:
- Remove the GM_simple and GM_ann model-type branches entirely; they
  were testing scaffolding. The only production path is nondim_rhoF_ann.
  This also removes the MESO_SFN_ANN_TYPE runtime parameter, the
  meso_sfn_ann_model_type CS field, and the now-unused local Kappa.
- Add a startup bounds check for MESO_SFN_ANN_WINDOW: the stencil
  shift must be <= 3 to stay inside the halo=3 provision in
  meso_sfn_ANN_compute, and the window must be an odd integer.
- Set and check the CS%initialized flag so using the module without
  calling _init produces a clear FATAL instead of later undefined
  behaviour.
- Rename lowercase runtime params to the MOM6 uppercase convention:
  meso_sfn_ann_coeff/type/window/file -> MESO_SFN_ANN_COEFF,
  MESO_SFN_ANN_WINDOW, MESO_SFN_ANN_FILE (type removed).
- Expand the module docstring with a Ford-style summary of the
  pipeline and an explicit warning that the dimensionalization in
  _compute is an implicit contract with the training procedure.
- Document the extended stencil bounds in the corner-point velocity
  gradient loop.

CI fixes:
- build-openmp: add Sfn_unlim_u_3D / Sfn_unlim_v_3D to the shared(...)
  clauses of the two !$OMP parallel do blocks in thickness_diffuse_full.
- check-style-and-docstrings: wrap the Fy_c declaration comment onto
  a second line so the line stays under the 120-char limit.

Breaking parameter changes (branch not yet released): MESO_SFN_ANN_TYPE
removed; the four lowercase params listed above renamed to uppercase.

Answer-changing only when USE_THICKNESS_DIFFUSE_ANN=True.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Resolve memory leak (deallocate)

Use faster ANN inference interface (ANN_apply_array_sio). 18% acceleration, regression is the same

* Changes to pass MOM6 testing system.

The tests passed:
test.grid
test.layout
test.restart
test.openmp
test.nan
test.dim

1) Trailing space is removed

2) Physical dimensionality is added to all float variables

3) Loop bounds are adjusted to get rid of out of bound memory access and pass test.grid

4) Scaling with physical units (i.e., structure US) is adjusted in multiple places to pass test.dim

5) Maximum allowed window size is limited to 3, which was tested now. Larger windows will likely needed calling pass_var (not yet implemented)

* Add proper interaction of buoyancy ANN with use_stored_slopes and SKEBs parameterizations

* Change default value of MESO_UPSILON_CLAMP parameter

Default values of limiting parameters are adjusted based on the diagnosis of CESM ocean model output.

MESO_UPSILON_CLAMP reduced from 1e4 m2 s-1 to 15 m2 s-1. This choice guarantees that the maximum overturning per one grid point and water column is approximately 1 Sv on CESM grid (before limiting and smoothing performed in MOM_thickness_diffuse.F90). The previous default value could allow up to ~600 Sv per grid point, which is an enormously large number.

MESO_SFN_FLUX_CLAMP, which is ~100 kg m-2 s-1, is likely never activated. Still, this number is reasonable as maximum predicted fluxes in CESM are ~20 kg m-2 s-1.

MESO_SFN_MAG_GRAD_FLOOR, which is 1.0e-10 kg m-4, is a very realistic number for CESM. The density gradients are approximately in a range from 1e-10 up to 1e-1, with a couple of grid points having 1e-12.

* Integrate pass_vector for sfn_u and sfn_v

Adding an exchange of halo points for the stream function predicted by the neural network. 

This halo exchange has no effect in simple geometries, but gaurantees Temperature/Salinity conservation on a tripolar grid, where rotational invariance is required to achieve conservation through the pole. Exact rotational invariance is not supported by the neural network. Rotational invariance is learned from data approximately, and is preserved less strictly than the numerical precision. Exchange of a streamfunction guarantees that there is no jump in the thickness flux at the grid point adjacent to the pole.

* Encapsulate use_meso_sfn_ANN in thickness_diffuse_CS

Address Hallberg's review comments about opaque-type encapsulation
(#1109).

- MOM.F90: Drop the if/else around the two thickness_diffuse calls
  and pass u, v unconditionally. The previous gating on
  CS%thickness_diffuse_CSp%use_meso_sfn_ANN peeked inside the
  opaque thickness_diffuse_CS type. The runtime check inside
  thickness_diffuse at line 503 (CS%use_meso_sfn_ANN .and.
  present_vel) still gates the ANN call correctly; present_vel
  is now always true but the conjunction is harmless.

- MOM_thickness_diffuse.F90: Make use_meso_sfn_ANN private
  (was incorrectly declared public). No external readers remain
  after the MOM.F90 refactor.

Expected to be bitwise identical based on inspection of the runtime
gate at MOM_thickness_diffuse.F90:503 (the conjunction
CS%use_meso_sfn_ANN .and. present_vel still routes the same way).
Not locally verified by test.repro / test.regression; CI will check.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Clarify drdx_u/drdy_v docstrings as gradients along z surfaces

Per Hallberg's review feedback on #1109: emphasize
in the Doxygen comments that the new drdx_u and drdy_v output
arguments of calc_isoneutral_slopes are evaluated along surfaces
of constant z (not along isopycnals or model interfaces).

All answers are bitwise identical.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* *Apply MOM6 soft-case convention in MOM_meso_sfn_ANN

Address Hallberg's style review on #1109:

- Loop variables: rename lowercase k -> K on interface-range
  loops (k=2,nz and k=1,nz+1) in meso_sfn_ANN_compute,
  center_grad_rho, and center2uv. Rename lowercase j -> J on
  v-point loops (j=js-1,je with v-point array access in the
  body). Update one inner u-loop (do i=is-1,ie -> do I=is-1,ie)
  to match the body's I usage.

- Array indexing: flip interface-staggered array accesses
  (drdx_c, drdy_c, drdx_u, drdy_v, drdz_u, drdz_v, Fx_c, Fy_c,
  Fx_u, Fy_v, sfn_u, sfn_v, var1_c, var2_c, var1_u, var2_v) to
  use uppercase K on their third index. Layer-staggered velocity
  arrays (dudx, dudy, dvdx, dvdy) keep lowercase k to signal
  "the layer below interface K".

- mag_grad expression: rewrite from the FMA-paranoid
  (US%Z_to_L*drd_*)*(US%Z_to_L*drd_*) form to Hallberg's
  preferred US%Z_to_L**2*drd_***2 form (he explicitly noted
  the extra parentheses are not needed for FMA/symmetry).

Answer-changing only when USE_THICKNESS_DIFFUSE_ANN=True. The
case-only changes are bit-identical by language definition (Fortran
is case-insensitive). The simplified mag_grad expression is
bit-different under FMA-aware compilers. Since the feature defaults
to .false. and has not been released, no _BUG flag is required.
Not locally verified; CI will check.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Drop optional from u,v in thickness_diffuse; fix v docstring

Address Hallberg's follow-up comments on #1109:

- Drop `, optional` from the u and v dummy arguments of
  thickness_diffuse. Both callers in MOM.F90 unconditionally pass
  u,v after the previous opaque-type refactor (98038be), so the
  optional attribute is no longer needed.

- Remove the now-redundant `present_vel` variable (declaration
  and assignment), and simplify the ANN call gate from
  `if (CS%use_meso_sfn_ANN .and. present_vel)` to
  `if (CS%use_meso_sfn_ANN)`.

- Restore the truncated `v` Doxygen description and tighten
  whitespace on both u and v declaration lines.

All answers are bitwise identical: `present_vel` was always true
after the prior refactor, so the conjunction always took the same
branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* Make find_eta halo size conditional on use_meso_sfn_ANN

Per Hallberg's review on #1109: avoid the cost of
the wider halo in find_eta for default users.

The ANN streamfunction needs the interface elevation `e` with a
wider halo to support its stencil. Previously the halo bump was
unconditional. Now it is gated on `CS%use_meso_sfn_ANN` so that
default users (USE_THICKNESS_DIFFUSE_ANN=False) retain the
original `halo_size=1` call.

Default-off users now match upstream dev/gfdl exactly at this
call site. ANN-on users are unchanged relative to the previous
branch state.

Hallberg also recommends verifying halo updates by comparing runs
at different MPI process counts (e.g., mpirun -n 63 vs -n 64);
that test has not yet been run. CI may catch some issues; the
cross-process-count check is a separate validation step.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Pavel <pperezhogin@gmail.com>
Co-authored-by: Pavel Perezhogin <35234405+Pperezhogin@users.noreply.github.com>
Building MOM_diag_mediator.F90 with GCC and -O2 became much slower after
diagnostic buffers were added.
```
    commit 6f6975a
    Author: Andrew Shao <andrew.shao@hpe.com>
    Date:   Wed Mar 18 12:17:43 2026 -0700

        +Extend diag_mediator to allow the piecemeal posting of diagnostics (#809)
```
Before this commit, MOM_diag_mediator.F90 built in about 15 seconds.
After this commit, build time increased to more than 170 seconds.

The slowdown was isolated to derived types with multiple allocatable
array components, not to abstract types, inheritance, or type-bound
methods.  A minimal form of the triggering pattern is shown below.
```
  type :: diag_buffer_2d
    real, allocatable :: buffer(:,:,:)
    integer, allocatable :: ids(:)
  end type
```
The component did not need to be referenced in MOM_diag_mediator.F90.
It was enough for diag_buffer_2d to appear as a component of axes_grp.

Somehow, this was resolved by defining an explicit dummy finalizer in
`diag_buffer_2d`.
```
  type :: diag_buffer_2d
    ! ...
  contains
    final :: finalize_diag_buffer_2d
  end type diag_buffer_2d

  subroutine finalize_diag_buffer_2d
    type(diag_buffer_2d), intent(inout) :: this
  end subroutine finalize_diag_buffer_2d
```
With this change, build time returns to about 15 seconds.

* The finalizers are intentionally empty.  Allocatable components are
  automatically deallocated by the language, so no explicit deallocate()
  calls are needed here.

  Finalizers should only be used for custom cleanup, such as external
  resources or manually managed pointer targets.

* `diag_buffer_[23]d` contains an array of `buffer_[23]d` objects.  The
  language standard notes that components are finalized before the
  parent.

* No similar compile-time benefit was seen from adding dummy finalizers
  to buffer_[23]d, so those types are left unchanged.
Previously, the nonlinear part of basal friction was evaluated
cell-wise, while the linear part was evaluated at quadrature points;
the resulting inconsistency in the Jacobian prevented quadratic
convergence when using Newton iterations, so moved all friction to be
evaluated evaluated at quadrature points instead. Viscosity should
also be evaluated at the 4 quad points rather than cell-centered, so
changed the default for NUMBER_OF_ICE_VISCOSITY_QUADRATURE_POINTS from
1 to 4 (which is more accurate anyway). These changes allow us to
achieve the expected quadratic convergence when using Newton.

With these changes, several deprecated cell-averaged ice-shelf CS
fields were removed: basal_traction, newton_umid, newton_vmid,
newton_drag_coef. Subroutine calc_shelf_taub was converted to return
area-averaged basal shear stress, now for diagnostic purposes only.

Also removed incorrect 0.5 factor from newton_visc_factor
-Added NEWTON_CONJUGATE_GRADIENT_TOLERANCE to set a separate tolerance
for Newton iterations compared to Picard. Helpful when using a higher
convergence tolerance for initial inexact Newton tolerances, or a
lower tolerance is used for "exact" Newton.
-Can now set NEWTON_AFTER_TOLERANCE<=0 to only use
Newton iterations to solve the SSA, bypassing Picard iterations
entirely. This works well if the initial guess is close to the
solution.

Further developed the Inexact Newton Eisenstat-Walker (EW) scheme:
-EW now evolves (correctly) based on the change in L2 norm of the
stress residual
-New parameters NEWTON_EW_GAMMA and NEWTON_EW_ALPHA to specify the EW
gamma and alpha instead of hardcoding them
-Added new EW safeguard based on PETSc, choice 3, along with
parameters:
 -NEWTON_EW_SAFETY: choose between the 2 safeguards
 -NEWTON_EW_1_THRES: threshold used for when NEWTON_EW_SAFETY is 1
 -NEWTON_EW_ETA_MAX: Max allowed EW eta (between 0 and 1)
All of these new parameters will typically work well at their
defaults, but are available for fine-tuning

Added new SSA convergence criteria based on L2 norms (needed for EW):
-NONLIN_SOLVE_ERR_MODE=4: err_mode for convergence based on nonlin
residual | F | / | F_0 |
-NONLIN_SOLVE_ERR_MODE=5: err_mode for convergence based on relative
residual | F | / | tau |
-SSA_ADD_REL_RESID = True: If NONLIN_SOLVE_ERR_MODE<=4, this will also
ensure convergence of the relative residual (via err_mode=5). While
generally useful for ensuring convergence of the solution (err_mode 2
or 3) and the residual simultaneously, in some cases, it is
essential. For example, if using a solution-based convergence
criterion with inexact Newton, initial Newton iterations may not
change the solution much and could trigger the stopping criterion
before the problem is actually converged; using SSA_ADD_REL_RESID
avoids this issue.
-ICE_RR_NONLINEAR_TOLERANCE: Tolerance for err_mode 5 if
SSA_ADD_REL_RESID=True.

Added several new MOM_mesg calls to track the adaptive inner solver
tolerance and the nonlinear relative residual.

Added VERBOSITY parameter to ice-shelf driver

Ice-shelf efficiency improvements

- Precomputed some commonly-used variables
- Replace some pass_var()/pass_vector() pairs with do_group_pass()
- Indentation

Add missing ice_shelf CS%rhow_rhoi substitutions
  Set a pointer to the `df_2d_y` optional argument in `register_tracer()`,
treating it like `df_2d_x` and the other diagnostic arrays that can be provided
as optional arrays to `register_tracer()`.  Any 2-d y-diffusion diagnostics
impacted by this bug would have reported all 0 values, but now will return
sensible values.  Almost all (perhaps all) tracers have their standard
diagnostics set up via `register_tracer_diagnostics()`, and this bug fix does
not impact them.  As a result, it is entirely possible that this bug does not
impact any code that is used, which is why it was never detected.

  Additionally, this commit comments out two unused and unimplemented
diagnostics (ear and ebr) from MOM_offline_main.  For now the registration calls
for these diagnostics are being left in comments in case someone actually
intends for them to exist, but if they are not actually implemented they should
be removed altogether at a later date.  These broken diagnostics were detected
while scanning for `register_diag_field()` calls that appear to be missing
appropriate unit conversion factors.  Without an actual implementation, there
would have been an error message about missing `post_data()` calls for these
diagnostics had they ever been turned on in the diag_table of an offline tracer
run, so they can not have been in use in any working simulations.

  All solutions are bitwise identical, but there will be changes in the
available_diags files for offline tracer runs.
When ODA_INCUPD=True, the NOTE "ended updating fields with increments. " was intended to be produced once per run but was instead being printed every time step after the increments were done.

The NOTE isn't necessary and so has been deleted.  Answers are unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.